Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hop nodes for k8s #13904

Merged
merged 45 commits into from
Aug 29, 2023
Merged

Hop nodes for k8s #13904

merged 45 commits into from
Aug 29, 2023

Conversation

fosterseth
Copy link
Member

@fosterseth fosterseth commented Apr 25, 2023

SUMMARY

Allow users to setup hop nodes for k8s deployments

  • revert using devel receptor-collection before merging
ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • API
AWX VERSION
awx: 22.0.1

awx/api/serializers.py Outdated Show resolved Hide resolved
awx/api/views/root.py Outdated Show resolved Hide resolved
@fosterseth fosterseth changed the title [WIP] Hop nodes for k8s Hop nodes for k8s Jul 12, 2023
@AlanCoding
Copy link
Member

in operator check:

Traceback (most recent call last):
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/tasks/jobs.py\", line 597, in run
    res = receptor_job.run()
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/tasks/receptor.py\", line 318, in run
    res = self._run_internal(receptor_ctl)
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/tasks/receptor.py\", line 393, in _run_internal
    res = processor_future.result()
  File \"/usr/lib64/python3.9/concurrent/futures/_base.py\", line 446, in result
    return self.__get_result()
  File \"/usr/lib64/python3.9/concurrent/futures/_base.py\", line 391, in __get_result
    raise self._exception
  File \"/usr/lib64/python3.9/concurrent/futures/thread.py\", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/utils/common.py\", line 1189, in wrapper_cleanup_new_process
    return func(*args, **kwargs)
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/tasks/receptor.py\", line 461, in processor
    return ansible_runner.interface.run(
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/ansible_runner/interface.py\", line 210, in run
    r = init_runner(**kwargs)
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/ansible_runner/interface.py\", line 107, in init_runner
    stream_processor = Processor(event_handler=event_callback_handler,
  File \"/var/lib/awx/venv/awx/lib64/python3.9/site-packages/ansible_runner/streaming.py\", line 270, in __init__
    self.artifact_dir = os.path.join(project_artifacts, ident)
  File \"/usr/lib64/python3.9/posixpath.py\", line 90, in join
    genericpath._check_arg_types('join', a, *p)
  File \"/usr/lib64/python3.9/genericpath.py\", line 152, in _check_arg_types
    raise TypeError(f'{funcname}() argument must be str, bytes, or '
TypeError: join() argument must be str, bytes, or os.PathLike object, not 'int'

This is fixed with ansible/ansible-runner#1268

awx/main/models/ha.py Outdated Show resolved Hide resolved
fosterseth and others added 16 commits August 23, 2023 22:05
Dynamically flipping from Established
to Disconnected is not the intended
usage of InstanceLink State.

- Link state starts in Adding and becomes
Established once any control node first sees the link
is in the status KnownConnectionCosts
API changes
- cannot change peers or enable
peers_from_control_nodes on VM deployments
- allow setting ip_address
- use ip_address over hostname in the generated
group_vars/all.yml
- Drop api/v2/peers endpoint

DB changes
- add ip_address unique constraint, but ignore "" entries

Other changes
- provision_instance should take listener_port option

Tests
- test that new controls doesn't disturb other peers
relationships
- test ip_address over hostname
Get rid of PeersSerializer and just use SlugRelatedField,
which should be more a straightforward approach.

Other changes:
- cleanup code related to the already-removed api/v2/peers
endpoint
- add "hybrid" node type into more instance_peers test cases
Add a check_peers_changed() utility method
to determine if peers in attrs matches
the current instance peers.

Other changes:
- Set ip_address default to "", and do not
allow null.
Setting a different value for ip_address
and hostname does not work with the current
way we create receptor certs.
Make test case cleaner by using itertools product
instead of the triple nested loop

Replace triple single quotes with triple
double quotes
Might help to install receptor last,
that way when nodes are first connected to the mesh
they already have podman installed and can potentially
run jobs. Otherwise it might be possible for controller
to launch jobs against nodes that aren't fully set up.
receptor_host_identifier can be left out
of group_vars and will default to the
'ansible_host' variable
@@ -7,14 +7,11 @@
user:
name: "{{ receptor_user }}"
shell: /bin/bash
- name: Enable Copr repo for Receptor
command: dnf copr enable ansible-awx/receptor -y
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be removed from previous installed receptors?

Copy link
Member Author

@fosterseth fosterseth Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, can you clarify the question? The next receptor-collection version will install receptor via these releases by default https://github.com/ansible/receptor/releases

thus the Copr repo is no longer needed going forward

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking of upgrades of previous installs.

version: 1.1.0
- name: https://github.com/ansible/receptor-collection.git
type: git
version: main
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to point this to 2.0.0 version?

@@ -40,7 +40,7 @@ echo "Admin password: ${DJANGO_SUPERUSER_PASSWORD}"
awx-manage create_preload_data
awx-manage register_default_execution_environments

awx-manage provision_instance --hostname="$(hostname)" --node_type="$MAIN_NODE_TYPE"
awx-manage provision_instance --hostname="$(hostname)" --node_type="$MAIN_NODE_TYPE" --listener_port=2222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This concerns me a little bit about the expected practice. Does this need to be provided for VM installs in general? I don't get why we didn't need this before, but would now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems that this is not required, and as such, it would make the most sense to not set it here, because the VM install scenario that this corresponds to should not ordinarily be setting the listener port.

@AlanCoding
Copy link
Member

Filed a followup issue #14387

@fosterseth fosterseth merged commit c9190eb into devel Aug 29, 2023
15 of 16 checks passed
@fosterseth fosterseth deleted the feature_hop-node branch August 29, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api component:awx_collection issues related to the collection for controlling AWX component:docs component:ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.